[Feat] 세모피드 이모지 클릭 시 FCM 알림 추가#142
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 본 PR은 사용자가 세모피드에 이모지 반응을 남길 때 작성자에게 FCM 알림을 발송하는 기능을 구현합니다. 반응 등록 시에만 알림이 발송되도록 제어하며, 불필요한 알림 방지를 위해 본인 반응이나 탈퇴한 사용자에 대한 예외 처리를 포함합니다. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new notification type SEMOFEED_EMOJI and the SemoFeedNotificationService to notify feed authors when their posts receive emoji reactions. The notification logic is integrated into SemoFeedEmojiService and covered by unit tests. The review feedback highlights two main issues: first, the notification dispatch is coupled with the main emoji toggle transaction, which could cause rollback propagation if the notification fails (violating the repository style guide); second, there is a duplicate database query checking user existence in both the notification service and the underlying send method.
| if (reacted) { | ||
| // 새 반응 등록일 때만 작성자 알림을 발송합니다. | ||
| semoFeedNotificationService.sendEmojiNotification(semoFeed, user, emojiType); | ||
| } |
There was a problem hiding this comment.
현재 SemoFeedEmojiService.toggleWithCount 메서드 내에서 이모지 토글 트랜잭션과 알림 발송(SemoFeedNotificationService.sendEmojiNotification)이 동기적으로 처리되고 있습니다.
이 구조는 다음과 같은 문제를 야기할 수 있습니다:
- 트랜잭션 롤백 전파 (Repository Style Guide Rule 29 위반): 알림 저장이나 FCM 토큰 조회 등 알림 발송 과정에서 예외가 발생하면 주 비즈니스 로직인 이모지 반응 등록까지 함께 롤백됩니다. 알림은 부가적인 기능(Side Effect)이므로 실패하더라도 주 비즈니스 로직에 영향을 주지 않아야 합니다.
- 성능 저하: 알림 발송을 위한 추가적인 DB 조회 및 저장 작업이 주 트랜잭션 내에서 실행되므로, 커넥션 점유 시간이 길어지고 성능 병목이 발생할 수 있습니다.
개선 제안:
Spring의 ApplicationEventPublisher를 사용하여 이모지 반응 등록 이벤트를 발행하고, 이를 @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)로 수신하여 처리하는 구조로 개선하는 것을 권장합니다. 이렇게 하면 주 트랜잭션이 성공적으로 커밋된 후에만 알림 로직이 실행되므로, 알림 실패가 주 비즈니스 로직에 영향을 주지 않고 성능도 개선됩니다.
References
- 알림, 메시징, FCM 등 외부 사이드 이펙트 처리 시 실패가 주 비즈니스 작업을 의도치 않게 롤백하지 않도록 검증해야 합니다. (link)
| if (!userRepository.existsByIdAndDeletedFalse(receiver.getId())) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
userRepository.existsByIdAndDeletedFalse(receiver.getId()) 호출로 인해 중복 쿼리가 발생합니다.
NotificationService.send 메서드 내부(46라인)에서도 동일하게 userRepository.existsByIdAndDeletedFalse(receiverId)를 호출하여 수신자의 존재 여부 및 탈퇴 여부를 검증하고 있습니다. 이로 인해 동일한 receiverId에 대해 exists 쿼리가 두 번 연속으로 실행되는 비효율이 존재합니다.
개선 제안:
알림 발송 실패(탈퇴한 사용자 등)가 주 비즈니스 트랜잭션에 영향을 주지 않도록 이벤트를 통해 비동기/분리 처리하거나, NotificationService에 예외를 던지지 않고 무시하는 안전한 발송 메서드(예: sendIfPresent)를 추가하여 중복 쿼리를 제거하는 것을 고려해 주세요.
1 participant
🧾 요약
🔗 이슈
✨ 변경 내용
SEMOFEED_EMOJI추가actorId,actorName,semoFeedId,emojiType포함 및 필수 값 검증✅ 확인